Open
Conversation
- Add new test scenario for leveraged loop staked ether - Configure SLLSE fork in Scarb.toml for mainnet block 1794270 - Update lib.cairo to include new scenario module
…mplementation - Upgrade OpenZeppelin from v2.0.0 to v3.0.0-alpha.1 using git dependency - Remove custom oz_4626.cairo implementation (deprecated) - Update all package dependencies for consistency - Add pragma integration interface for price feeds - Refactor vault and allocator components to use new OZ interfaces - Update test scenarios and middleware implementations
Initialize NestJS-based backend with API, indexer, and relayer services. Includes shared libraries for config, database, logging, and core types.
- Enhanced @forge/logger with Winston-based structured logging - Added daily log rotation, error-specific files, and JSON/simple formats - Implemented comprehensive logging in indexer service with event tracking - Added production-ready logging to API service with request tracing - Removed deprecated relayer and core packages - Updated package dependencies and configurations - Added Docker support with API and indexer containers - Enhanced environment validation and configuration management
- Remove unused vault_allocator field from storage in multiple decoder components - Add new ModifyPositionParamsV2 and ModifyLever types for V2 integration - Clean up transfer_position functionality in Vesu decoder - Add multiply and vesu_v2 decoder sanitizer modules to lib.cairo - Reorganize test structure and rename vesu integration test to vesu_v1 - Remove deprecated test creator module
- Add CallPerformed event to track all contract calls made through manage functions - Refactor manage and manage_multi to use new call_contract helper - Emit detailed call information including contract address, selector, calldata, and results
- Add configurable rate limiting with period-based call counting - Improve swap logic with proper zero amount handling and math utilities - Update decoder types to support AmountV2 with denomination field - Add comprehensive error handling for rate limits and validation - Refactor slippage calculation using math library for better precision
- Remove unused RedeemRequestInfo import from vault interface - Fix price calculation using u256_mul_div with ceiling rounding in price router
- Add deployment files exclusion to .gitignore - Include .env in gitignore - Add scripts directory with deployment utilities
- Add bring_liquidity method to base decoder and sanitizer interface - Implement bring_liquidity in BaseDecoderAndSanitizerComponent - Add mock vault module and deploy utility function - Create vault bring liquidity integration test structure - Add vault allocator leaf generation utilities for approvals and bring liquidity operations - Update module exports to include new components
- Add development Docker setup with dedicated dev compose file - Configure nodemon for hot reload in API and indexer services - Add FORCE_START_BLOCK option for flexible indexer restart - Improve error handling in Prisma service for missing tables - Add decimal.js dependency for precise financial calculations - Update environment validation with proper types - Enhance logging consistency across services - Add comprehensive Docker command orchestration - Update README with backend and scripts documentation sections
There was a problem hiding this comment.
Pull Request Overview
This PR appears to be a comprehensive update to the StarkNet Vault Kit codebase, primarily addressing OpenZeppelin v6 compatibility and introducing new functionality. The changes involve updating import paths, replacing deprecated interfaces, and adding new deployment/management scripts.
- Updates all OpenZeppelin imports to v6 interface paths (e.g.,
openzeppelin::interfaces::*instead ofopenzeppelin::token::*::interface) - Removes custom ERC4626 implementation in favor of OpenZeppelin's updated version
- Adds comprehensive deployment and configuration scripts in the
scripts/directory
Reviewed Changes
Copilot reviewed 117 out of 121 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| scripts/* | Complete deployment and management script infrastructure with interactive configuration |
| packages/vault_allocator/src/**/*.cairo | Updated OpenZeppelin v6 imports and interface compatibility |
| packages/vault/src/**/*.cairo | Updated imports and removed custom ERC4626 implementation |
| packages/vault/src/oz_4626.cairo | Removed custom ERC4626 implementation entirely |
Files not reviewed (1)
- scripts/pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| fn manage(ref self: ContractState, call: Call) -> Span<felt252> { | ||
| self._only_manager(); | ||
| call_contract_syscall(call.to, call.selector, call.calldata).unwrap_syscall() | ||
| self.call_contract(call.to, call.selector, call.calldata) |
There was a problem hiding this comment.
The direct call_contract_syscall has been replaced with a new internal method call_contract that adds event emission. This improves observability but ensure the new method maintains the same error handling behavior as the original syscall.
|
|
||
| fn multi_route_swap( | ||
| self: @ContractState, | ||
| ref self: ContractState, |
There was a problem hiding this comment.
The multi_route_swap function signature has been changed from self: @ContractState to ref self: ContractState. This is a breaking change that allows the function to modify state (likely for rate limiting), but callers will need to be updated to handle mutable references.
Comment on lines
+139
to
+140
| if (sell_token_amount == Zero::zero()) { | ||
| return Zero::zero(); |
There was a problem hiding this comment.
[nitpick] Added early return for zero amount swaps, which is good defensive programming. However, consider whether this should emit an event or revert instead, as zero-amount swaps might indicate a logical error in the caller.
Suggested change
| if (sell_token_amount == Zero::zero()) { | |
| return Zero::zero(); | |
| Errors::zero_amount_swap(); |
packages/vault/src/vault/vault.cairo
Outdated
Comment on lines
33
to
34
| DefaultConfig, ERC4626Component, ERC4626DefaultNoFees, ERC4626DefaultNoLimits, | ||
| }; |
There was a problem hiding this comment.
The code now uses OpenZeppelin's built-in ERC4626 implementation instead of the custom oz_4626 module that was removed. Ensure that all functionality from the custom implementation is preserved in the OpenZeppelin version.
Suggested change
| DefaultConfig, ERC4626Component, ERC4626DefaultNoFees, ERC4626DefaultNoLimits, | |
| }; | |
| DefaultConfig, ERC4626Component, | |
| }; | |
| // --- Custom ERC4626 Configs for Fees and Limits --- | |
| // If the removed oz_4626 module had custom fee/limit logic, define them here. | |
| #[derive(Copy, Drop, Serde)] | |
| struct CustomFees { | |
| management_fee_bps: u16, | |
| performance_fee_bps: u16, | |
| redemption_fee_bps: u16, | |
| } | |
| #[derive(Copy, Drop, Serde)] | |
| struct CustomLimits { | |
| max_deposit: u256, | |
| max_withdraw: u256, | |
| } | |
| // Implement custom config traits if needed | |
| impl ERC4626Component::Fee for CustomFees { | |
| fn management_fee(&self) -> u16 { | |
| self.management_fee_bps | |
| } | |
| fn performance_fee(&self) -> u16 { | |
| self.performance_fee_bps | |
| } | |
| fn redemption_fee(&self) -> u16 { | |
| self.redemption_fee_bps | |
| } | |
| } | |
| impl ERC4626Component::Limit for CustomLimits { | |
| fn max_deposit(&self) -> u256 { | |
| self.max_deposit | |
| } | |
| fn max_withdraw(&self) -> u256 { | |
| self.max_withdraw | |
| } | |
| } |
Comment on lines
+94
to
+100
| async function getVaultAddress(): Promise<string> { | ||
| const vaultAddress = await askQuestion("Enter vault contract address: "); | ||
| try { | ||
| validateAndParseAddress(vaultAddress); | ||
| return vaultAddress; | ||
| } catch (error) { | ||
| throw new Error(`Invalid vault address: ${vaultAddress}`); |
There was a problem hiding this comment.
The error handling discards the original validation error details. Consider preserving the original error message: throw new Error(Invalid vault address: ${vaultAddress}. ${error.message});
Suggested change
| async function getVaultAddress(): Promise<string> { | |
| const vaultAddress = await askQuestion("Enter vault contract address: "); | |
| try { | |
| validateAndParseAddress(vaultAddress); | |
| return vaultAddress; | |
| } catch (error) { | |
| throw new Error(`Invalid vault address: ${vaultAddress}`); | |
| const errorMsg = (error instanceof Error) ? error.message : String(error); | |
| throw new Error(`Invalid vault address: ${vaultAddress}. ${errorMsg}`); |
| vaultConfig.maxDeltaPercentage | ||
| ); | ||
| console.log("\n⏳ Waiting 3 seconds before deploying RedeemRequest..."); | ||
| await new Promise(resolve => setTimeout(resolve, 3000)); |
There was a problem hiding this comment.
[nitpick] Hard-coded 3-second delays are used throughout deployment. Consider making this configurable or using a more robust waiting mechanism that checks transaction confirmation rather than arbitrary time delays.
- Enhance base AUM provider with improved interface definitions - Update 4626 AUM provider implementation - Refactor AUM provider interface for better compatibility
- Update vault configuration with new settings - Improve script dependencies and configuration management - Enhance deployment configuration structure
- Implement client SDK for vault interactions - Add comprehensive API client functionality - Include type definitions and utilities for external integrations
- Restructure backend with improved microservices architecture - Add relayer services for automatic redeem and on-chain AUM - Improve pending redeems logic to handle claimed redeems properly - Update database service with better redeem filtering - Add comprehensive logging and configuration management - Remove scientific notation from decimal formatting - Add Docker development setup with service isolation
- Document incomplete relayer services with clear warnings - Update API endpoints count and descriptions to match implementation - Add comprehensive "What's Missing / TODO" section covering: - Incomplete services (automatic redeem and on-chain AUM relayers) - Missing features (auth, rate limiting, caching, monitoring, testing) - Configuration gaps (env validation, secrets management) - Update Docker instructions for development setup - Improve library descriptions with actual functionality
- Add new event structure with caller, amount, and state info - Emit event when liquidity is brought back from allocators - Update vault hash in config after contract changes
- Fix all Dockerfiles to properly handle workspace dependencies - Remove pnpm prune --prod to preserve all dependencies - Add symlinks for missing dependencies (decimal.js, starknet, @apibara packages, cron) - Fix API service port exposure (3000 instead of 3001) - Add complete docker-compose.yml with all services uncommented - Add missing environment variables for onchain-aum-provider - Ensure proper file permissions and user setup 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Add decimal.js and starknet symlinks to resolve runtime dependency issues in indexer and relayerOnChainAum containers.
- Remove flashloan interface and implementation from manager - Remove flashloan decoder/sanitizer methods - Remove flashloan mock and related test scenarios - Clean up unused Vesu integration interfaces - Simplify manager constructor by removing vesu_singleton dependency This refactoring removes the complex flashloan mechanism and associated test infrastructure, streamlining the vault allocator architecture.
- Remove vesu_singleton parameter from manager constructor and tests - Add epoch field to BringLiquidity event in vault - Re-enable commented test modules in vault_allocator lib - Clean up unused imports and variables across test files
- Add ERC721Enumerable component to redeem request contract - Implement due_assets_from_owner function for tracking user redemptions - Add new starknet_vault_kit decoder and sanitizer components - Introduce comprehensive merkle tree system with base functionality - Add integration modules for AVNU, ERC4626, and Vesu protocols - Clean up decoder components by removing unused dependencies - Remove obsolete register.cairo test file
- Remove legacy vesu.cairo interface - Split into vesu_v1.cairo and vesu_v2.cairo for version-specific implementations - Add price_router_vesu module for Vesu-specific price routing - Update lib.cairo to reflect new module structure
- rename vault to vault_allocator - fix svk replacing withdraw by mint
Implements OpenZeppelin's UpgradeableComponent to enable contract upgrades: - Added IUpgradeable interface and UpgradeableComponent - Added upgradeable storage and events - Owner-only upgrade function following existing pattern 🤖 Generated with [Claude Code](https://claude.ai/code)
- Remove VesuV2 component from SimpleDecoderAndSanitizer to avoid selector conflicts - Add new standalone VesuV2SpecificDecoderAndSanitizer contract - Update config with new contract hash - Add declaration script support for new contract - Update package.json scripts for separate decoder contracts
feat: configurable ERC4626LimitConfigImpl
- Fix import ordering and code formatting consistency - Add caller_is_not_vault_allocator error function - Implement authorization check in bring_liquidity function - Add test for unauthorized bring_liquidity access - Clean up whitespace and line formatting throughout test file
- Replace slot-based call tracking with time window approach - Simplify storage by using single window counter instead of map - Clean up parameter naming and code organization - Reset counter when window changes for accurate rate limiting
- Clean up unused imports in AVNU middleware - Update mainnet contract hashes in config.json - Rename SDK package to @forge_yields/starknet_vault_kit_sdk - Add yarn.lock for SDK dependencies
akiraonstarknet
force-pushed
the
main
branch
from
7ea29a1 to
9d3d2b4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.